-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: scalar timestamp assignment (#19843) #19973
Conversation
from pandas.core.dtypes.dtypes import DatetimeTZDtype | ||
|
||
|
||
# referencing #19843: scalar assignment of a tz-aware is object dtype |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can find a home for this in another test file (and don't need a new one). In addition, the comment should be under the class declaration.
@jreback : I think tests/frame/test_indexing.py
makes sense here. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, should they go under TestDataFrameIndexingDatetimeWithTZ
, or in their own class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same class sounds good for now.
@@ -3042,6 +3044,22 @@ def test_transpose(self): | |||
expected.index = ['A', 'B'] | |||
assert_frame_equal(result, expected) | |||
|
|||
def test_scalar_assignment(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reference issue number for both tests.
what should I do for the |
Perhaps something like:
|
Codecov Report
@@ Coverage Diff @@
## master #19973 +/- ##
==========================================
+ Coverage 92.06% 92.06% +<.01%
==========================================
Files 169 169
Lines 50681 50690 +9
==========================================
+ Hits 46659 46668 +9
Misses 4022 4022
Continue to review full report at Codecov.
|
doc/source/whatsnew/v0.23.0.txt
Outdated
@@ -854,6 +854,7 @@ Datetimelike | |||
- Bug in :func:`to_datetime` where passing an out-of-bounds datetime with ``errors='coerce'`` and ``utc=True`` would raise ``OutOfBoundsDatetime`` instead of parsing to ``NaT`` (:issue:`19612`) | |||
- Bug in :class:`DatetimeIndex` and :class:`TimedeltaIndex` addition and subtraction where name of the returned object was not always set consistently. (:issue:`19744`) | |||
- Bug in :class:`DatetimeIndex` and :class:`TimedeltaIndex` addition and subtraction where operations with numpy arrays raised ``TypeError`` (:issue:`19847`) | |||
- Bug in `DataFrame` assignment with a timezone-aware object (:issue:`19843`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use double-backticks are DataFrame
say a timezone-aware scalar
pandas/core/frame.py
Outdated
@@ -2868,9 +2868,15 @@ def reindexer(value): | |||
value = maybe_infer_to_datetimelike(value) | |||
|
|||
else: | |||
# upcast the scalar | |||
# cast ignores pandas dtypes. so save the dtype first | |||
from pandas.core.dtypes.cast import infer_dtype_from_scalar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import at the top of the file with other imports
pandas/core/frame.py
Outdated
# upcast the scalar | ||
# cast ignores pandas dtypes. so save the dtype first | ||
from pandas.core.dtypes.cast import infer_dtype_from_scalar | ||
pd_dtype, _ = infer_dtype_from_scalar(value, pandas_dtype=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
call the return dtype
pandas/core/frame.py
Outdated
value = cast_scalar_to_array(len(self.index), value) | ||
value = maybe_cast_to_datetime(value, value.dtype) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need the blank line and the comment here
pandas/tests/frame/test_indexing.py
Outdated
# issue #19843 | ||
df = pd.DataFrame(index=(0, 1, 2)) | ||
df['now'] = pd.Timestamp('20130101', tz='UTC') | ||
assert isinstance(df.dtypes[0], DatetimeTZDtype) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
construct the expected frame and use assert_frame_equal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
put this in class TestDataFrameIndexingDatetimeWithTZ(TestData)
name it test_setitem_scalar
parameterize it with both a tz and a tz-aware value.
pandas/tests/frame/test_indexing.py
Outdated
df['now'] = pd.Timestamp('20130101', tz='UTC') | ||
assert isinstance(df.dtypes[0], DatetimeTZDtype) | ||
|
||
def test_datetime_index_assignment(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a duplicate test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could put it as an additional test in the class as above if you'd like (parametrize as well)
can you rebase and update |
can you rebase & move the note to 0.24.0 |
fixed up, but going to do in |
* master: (47 commits) Run tests in conda build [ci skip] (pandas-dev#22190) TST: Check DatetimeIndex.drop on DST boundary (pandas-dev#22165) CI: Fix Travis failures due to lint.sh on pandas/core/strings.py (pandas-dev#22184) Documentation: typo fixes in MultiIndex / Advanced Indexing (pandas-dev#22179) DOC: added .join to 'see also' in Series.str.cat (pandas-dev#22175) DOC: updated Series.str.contains see also section (pandas-dev#22176) 0.23.4 whatsnew (pandas-dev#22177) fix: scalar timestamp assignment (pandas-dev#19843) (pandas-dev#19973) BUG: Fix get dummies unicode error (pandas-dev#22131) Fixed py36-only syntax [ci skip] (pandas-dev#22167) DEPR: pd.read_table (pandas-dev#21954) DEPR: Removing previously deprecated datetools module (pandas-dev#6581) (pandas-dev#19119) BUG: Matplotlib scatter datetime (pandas-dev#22039) CLN: Use public method to capture UTC offsets (pandas-dev#22164) implement tslibs/src to make tslibs self-contained (pandas-dev#22152) Fix categorical from codes nan 21767 (pandas-dev#21775) BUG: Better handling of invalid na_option argument for groupby.rank(pandas-dev#22124) (pandas-dev#22125) use memoryviews instead of ndarrays (pandas-dev#22147) Remove depr. warning in SeriesGroupBy.count (pandas-dev#22155) API: Default to_* methods to compression='infer' (pandas-dev#22011) ...
git diff upstream/master -u -- "*.py" | flake8 --diff